Skip to content

LibGfx: Include all contours in PathImplSkia::length() - #11049

Merged
gmta merged 1 commit into
LadybirdBrowser:masterfrom
Calme1709:path-length-all-contours
Aug 9, 2026
Merged

LibGfx: Include all contours in PathImplSkia::length()#11049
gmta merged 1 commit into
LadybirdBrowser:masterfrom
Calme1709:path-length-all-contours

Conversation

@Calme1709

@Calme1709 Calme1709 commented Aug 9, 2026

Copy link
Copy Markdown
Member

No description provided.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 05974672-1e70-445e-af9a-ab5ce3b8086a

📥 Commits

Reviewing files that changed from the base of the PR and between abdb9c5 and f4b7047.

📒 Files selected for processing (3)
  • Libraries/LibGfx/PathSkia.cpp
  • Tests/LibWeb/Text/expected/SVG/svg-path-get-total-length-multiple-subpaths.txt
  • Tests/LibWeb/Text/input/SVG/svg-path-get-total-length-multiple-subpaths.html
📝 Walkthrough

Walkthrough

PathImplSkia::length() now returns the combined length of all path contours. A new SVG test verifies getTotalLength() for a path with two subpaths.

Changes

SVG total length

Layer / File(s) Summary
Sum contour lengths and validate multiple subpaths
Libraries/LibGfx/PathSkia.cpp, Tests/LibWeb/Text/input/SVG/svg-path-get-total-length-multiple-subpaths.html, Tests/LibWeb/Text/expected/SVG/svg-path-get-total-length-multiple-subpaths.txt		 length() iterates over all contours and accumulates their lengths. The SVG test covers two subpaths and expects 120.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • LadybirdBrowser/ladybird#11006: The main PR directly extends the getTotalLength() functionality implemented in the retrieved PR by fixing PathImplSkia::length() to correctly sum multiple subpaths.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided, so its relevance to the changes cannot be assessed. Add a description that states the change to include all path contours in PathImplSkia::length() and the related SVG test.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gmta
gmta merged commit b826ab7 into LadybirdBrowser:master Aug 9, 2026
15 checks passed
@Calme1709
Calme1709 deleted the path-length-all-contours branch August 9, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmta gmta gmta approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Calme1709 @gmta